Proposal: Add error field to StageRetrying event (Section 9.6)#4
Open
samueljklee wants to merge 1 commit intostrongdm:mainfrom
Open
Proposal: Add error field to StageRetrying event (Section 9.6)#4samueljklee wants to merge 1 commit intostrongdm:mainfrom
samueljklee wants to merge 1 commit intostrongdm:mainfrom
Conversation
Add `error` parameter to `StageRetrying(name, index, attempt, delay, error)` so consumers can see why a stage is retrying, consistent with `StageFailed` which already surfaces `error`. Motivation: Outcome.failure_reason (Section 5.2) is populated for RETRY status but StageRetrying did not expose it, making verbose output less informative than it could be. Reference: samueljklee/attractor#36 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
Author
|
@jmccarthy Hi -- we ran into this while implementing the Section 9.6 event system. Users couldn't see why stages were retrying since StageRetrying only carries attempt and delay. The error is already available from Outcome.failure_reason at the retry point -- this just surfaces it in the event. Would love your thoughts. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Add an
errorfield to theStageRetryingevent type in Section 9.6, so consumers can see why a stage is retrying.Motivation
While implementing the Section 9.6 event system, a user reported their pipeline appearing to hang (samueljklee/attractor#36). After adding
--verboseoutput wired to the event system, the user could see stages retrying but not why:Their feedback: "It would be informative if it also showed me why it is retrying."
The gap
The
Outcomemodel (Section 5.2, line 1086) definesfailure_reasonfor bothFAILandRETRYstatuses:StageFailed(line 1622) already surfaces this aserror:But
StageRetrying(line 1623) does not, even thoughfailure_reasonis available at the retry point.Proposed change
With this change, verbose output becomes:
Notes
StageFailedwhich already haserrorOutcome.failure_reasonis already populated forRETRYstatus per Section 5.2Reference: samueljklee/attractor#36
🤖 Generated with Amplifier